[jni] Fix the context when using multiple flutter engines #2494
Conversation
PR HealthBreaking changes ✔️
This check can be disabled by tagging the PR with Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
This check can be disabled by tagging the PR with License Headers ✔️
All source files should start with a license header. Unrelated files missing license headers
This check can be disabled by tagging the PR with |
HosseinYousefi
force-pushed
the
fix-context
branch
from
dbbcc31 to
9eb6750
Compare
github-actions
bot
added
package:jnigen
type-infra
HosseinYousefi
force-pushed
the
fix-context
branch
from
cf24d52 to
ea24e4d
Compare
| /// activity.release(); | ||
| /// } | ||
| /// } | ||
| /// ``` |
There was a problem hiding this comment.
These rules are similar to the rules for autorelease pools. I wonder if a similar API would work here, to make this less of a footgun:
static void withAndroidActivity(int engineId, void Function(JObject?) callback) {
final activity = JniPlugin.getActivity(engineId);
callback(activity);
activity?.release();
}Though this would look a bit weird, and I'm not sure if the marginal safety improvement is worth it. It's still possible to pass an async callback and use the activity after it's freed. You'd probably need some way of checking that the runtime type of the callback returns void to be fully safe (hack: assert(callback.runtimeType.endsWith(' void')) can distinguish "() => void" from "() => Future<void>"). This was less of a problem for the autorelease pools case.
There was a problem hiding this comment.
Yeah I thought that's hacky and you can still do some Isolate.run in the middle of it.
Uses a map on the Java side to store all the plugin instances with the key of engineId. On the Dart side, we use the engineId to access to correct Flutter Engine's context even when there are multiple. Same goes for the activity. Since activity is a more volatile thing, documentation has been added to warn users about the limitations of this.
There really isn't a great way to test this via integration tests (having multiple flutter engines).